Skip to content

Conversation

@Fluf22
Copy link
Collaborator

@Fluf22 Fluf22 commented Mar 17, 2025

🧭 What and Why

🎟 JIRA Ticket: -

Changes included:

  • data can be None in request_options

@Fluf22 Fluf22 requested a review from a team as a code owner March 17, 2025 12:49
@Fluf22 Fluf22 requested review from morganleroi and shortcuts March 17, 2025 12:49
@Fluf22 Fluf22 self-assigned this Mar 17, 2025
@algolia-bot
Copy link
Collaborator

algolia-bot commented Mar 17, 2025

No code generated

If you believe code should've been generated, please, report the issue.

📊 Benchmark results

Benchmarks performed on the method using a mock server, the results might not reflect the real-world performance.

Language Req/s
python 1102


for key, value in _user_request_options.items():
request_options[key].update(value)
if request_options[key] is None:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

L68 there's a default for the None parameters, maybe do the same for data?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was hesitant because in other languages, if there are data set in the request options, we overwrite the original object.
It's not the case in python (yet), but I thought it was more safe to handle it carefully that way, to prevent any misbehaviour if we make the code more consistent with the other languages in the future

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we will go to that level of consistency honestly it seems too much but your solution works as well so i'm good with it!

@Fluf22 Fluf22 requested a review from shortcuts March 17, 2025 13:10
Copy link
Member

@shortcuts shortcuts left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ty and good catch!

@Fluf22 Fluf22 merged commit f5d130c into main Mar 17, 2025
16 checks passed
@Fluf22 Fluf22 deleted the fix/python-request-options branch March 17, 2025 13:13
algolia-bot added a commit to algolia/algoliasearch-client-python that referenced this pull request Mar 17, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants